Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Revert "idp/proxy: Match users by ID instead of name by default" #6420

Merged
merged 1 commit into from
Jun 1, 2023

Conversation

rhafer
Copy link
Contributor

@rhafer rhafer commented Jun 1, 2023

This reverts commit 52951b4.

The change broke authentication for at least the desktop client when using the builtin idp. There seem to be issues in the IDP (lico) which result in the implicit scoped not being added correctly in some case. When that scope is missing the lg.uuid claim will not be present in the userinfo and we can correctly match users by id.

This reverts back to the old behaviour of matching users by name. Which also brings some aspects of #904

Fixes #6415

@rhafer rhafer self-assigned this Jun 1, 2023
@update-docs
Copy link

update-docs bot commented Jun 1, 2023

Thanks for opening this pull request! The maintainers of this repository would appreciate it if you would create a changelog item based on your changes.

@ownclouders
Copy link
Contributor

E2E tests failed: https://drone.owncloud.com/owncloud/ocis/22846/69/1

💥 To see the trace, please open the link in the console ...

npx playwright show-trace https://cache.owncloud.com/public/owncloud/ocis/22846/tracing/admin-creates-group-admin-2023-6-1-06-42-57.zip

npx playwright show-trace https://cache.owncloud.com/public/owncloud/ocis/22846/tracing/admin-creates-user-admin-2023-6-1-06-56-24.zip

npx playwright show-trace https://cache.owncloud.com/public/owncloud/ocis/22846/tracing/admin-deletes-group-admin-2023-6-1-06-44-03.zip

npx playwright show-trace https://cache.owncloud.com/public/owncloud/ocis/22846/tracing/admin-user-can-change-personal-quotas-for-users-admin-2023-6-1-06-50-49.zip

npx playwright show-trace https://cache.owncloud.com/public/owncloud/ocis/22846/tracing/admin-user-can-change-personal-quotas-for-users-alice-2023-6-1-06-50-50.zip

npx playwright show-trace https://cache.owncloud.com/public/owncloud/ocis/22846/tracing/admin-user-can-change-personal-quotas-for-users-brian-2023-6-1-06-50-52.zip

npx playwright show-trace https://cache.owncloud.com/public/owncloud/ocis/22846/tracing/assign-user-to-groups-admin-2023-6-1-06-54-11.zip

npx playwright show-trace https://cache.owncloud.com/public/owncloud/ocis/22846/tracing/create-space-from-folder-alice-2023-6-1-06-58-55.zip

npx playwright show-trace https://cache.owncloud.com/public/owncloud/ocis/22846/tracing/create-space-from-resources-alice-2023-6-1-07-00-08.zip

npx playwright show-trace https://cache.owncloud.com/public/owncloud/ocis/22846/tracing/delete-user-admin-2023-6-1-06-55-18.zip

npx playwright show-trace https://cache.owncloud.com/public/owncloud/ocis/22846/tracing/edit-groups-admin-2023-6-1-06-45-10.zip

npx playwright show-trace https://cache.owncloud.com/public/owncloud/ocis/22846/tracing/edit-panel-can-be-opened-via-quick-action-and-context-menu-admin-2023-6-1-06-57-30.zip

npx playwright show-trace https://cache.owncloud.com/public/owncloud/ocis/22846/tracing/edit-user-admin-2023-6-1-06-53-05.zip

npx playwright show-trace https://cache.owncloud.com/public/owncloud/ocis/22846/tracing/list-members-via-sidebar-alice-2023-6-1-06-48-32.zip

npx playwright show-trace https://cache.owncloud.com/public/owncloud/ocis/22846/tracing/logo-can-be-changed-in-the-admin-settings-admin-2023-6-1-06-41-51.zip

npx playwright show-trace https://cache.owncloud.com/public/owncloud/ocis/22846/tracing/multiple-spaces-can-be-managed-at-once-in-the-admin-settings-via-the-batch-actions-alice-2023-6-1-06-47-24.zip

npx playwright show-trace https://cache.owncloud.com/public/owncloud/ocis/22846/tracing/spaces-can-be-managed-in-the-admin-settings-via-the-context-menu-alice-2023-6-1-06-46-17.zip

npx playwright show-trace https://cache.owncloud.com/public/owncloud/ocis/22846/tracing/unstructured-collection-of-testable-space-interactions-alice-2023-6-1-07-11-06.zip

npx playwright show-trace https://cache.owncloud.com/public/owncloud/ocis/22846/tracing/user-group-assignments-can-be-handled-via-batch-actions-admin-2023-6-1-06-51-58.zip

npx playwright show-trace https://cache.owncloud.com/public/owncloud/ocis/22846/tracing/user-login-can-be-managed-in-the-admin-settings-admin-2023-6-1-06-49-40.zip

npx playwright show-trace https://cache.owncloud.com/public/owncloud/ocis/22846/tracing/user-should-be-able-to-read-and-dismiss-notifications-alice-2023-6-1-07-03-33.zip

npx playwright show-trace https://cache.owncloud.com/public/owncloud/ocis/22846/tracing/user-should-be-able-to-read-and-dismiss-notifications-brian-2023-6-1-07-03-41.zip

npx playwright show-trace https://cache.owncloud.com/public/owncloud/ocis/22846/tracing/user-should-be-able-to-read-and-dismiss-notifications-carol-2023-6-1-07-04-44.zip

This reverts commit 52951b4.

The change broke authentication for at least the desktop client when
using the builtin idp. There seem to be issues in the IDP (lico) which
result in the implicit scoped not being added correctly in some case.
When that scope is missing the `lg.uuid` claim will not be present in
the userinfo and we can correctly match users by id.

This reverts back to the old behaviour of matching users by name. Which
also brings some aspects of owncloud#904

Fixes owncloud#6415
@sonarcloud
Copy link

sonarcloud bot commented Jun 1, 2023

Kudos, SonarCloud Quality Gate passed!    Quality Gate passed

Bug A 0 Bugs
Vulnerability A 0 Vulnerabilities
Security Hotspot A 0 Security Hotspots
Code Smell A 0 Code Smells

No Coverage information No Coverage information
0.0% 0.0% Duplication

@rhafer rhafer requested a review from kobergj June 1, 2023 08:41
@rhafer
Copy link
Contributor Author

rhafer commented Jun 1, 2023

Just did a quick manual test with the desktop client. It works again now.

@rhafer rhafer merged commit 7a4bc71 into owncloud:master Jun 1, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

[QA] OCIS: Error while trying to log in to OAuth2-enabled server.
3 participants